Add local HA operator status - #887
Conversation
🔐 Codex Security Review
Review SummaryOverall Risk: MEDIUM Findings[MEDIUM] HA status aborts when the local Fleet process is unavailable
NotesThe authoritative diff was well-formed. No concrete cryptostealing, authentication, command-injection, protobuf, or unsafe SQL issues were found in the changed hunks. Generated by Codex Security Review | |
There was a problem hiding this comment.
Pull request overview
This PR introduces a local-only, redacted HA operator status surface for Proto Fleet, adds a fleet-ha status CLI to read it (with optional deeper dependency checks), and tightens the VIP/nginx boundary so /health/ha diagnostics are not exposed publicly. It also adds the running release version to the public /health response via the X-Proto-Fleet-Version header.
Changes:
- Add
/health/haloopback handler returning a redacted HA runtime contract, and includeX-Proto-Fleet-Versionon/health. - Add
fleet-ha status [node.env] [--json] [--check]which reads the local HA status and optionally checks etcd/Patroni/DB writer/VIP readiness. - Update HA runtime/coordinator snapshot semantics and deployment/nginx/RFC docs to match the local-only diagnostic boundary.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| server/internal/handlers/health/handler.go | Add version header on /health; add /health/ha JSON handler for redacted HA status. |
| server/internal/handlers/health/handler_test.go | Add tests for /health/ha redaction and /health version header. |
| server/internal/ha/status.go | Define redacted HA status contract and implement Runtime.Status(). |
| server/internal/ha/status_test.go | Add unit tests covering status transitions and redaction semantics. |
| server/internal/ha/runtime.go | Extend runtime owner interface to expose a Snapshot() for status reporting. |
| server/internal/ha/runtime_test.go | Update runtime owner test fakes to satisfy Snapshot() interface. |
| server/internal/ha/deployment/status.go | Implement fleet-ha status runtime read + optional control-path dependency probes. |
| server/internal/ha/deployment/preflight.go | Refactor Fleet env parsing into loadFleetEnvironment and strengthen required key validation. |
| server/internal/ha/coordinator.go | Rework snapshot fields to support freshness/availability without leaking error details. |
| server/internal/ha/config.go | Export LoadServiceTLS for shared use between runtime and host tooling. |
| server/cmd/fleetd/main.go | Wire version header handler and mount /health/ha only when HA is enabled. |
| server/cmd/fleetd/main_test.go | Add test ensuring HA requires loopback-only HTTP listen address. |
| server/cmd/fleetd/config.go | Add validateHAHTTPAddress enforcing loopback listen address when HA enabled. |
| server/cmd/fleet-ha/main.go | Add status subcommand with human/JSON output and --check failover readiness gate. |
| server/cmd/fleet-ha/main_test.go | Add CLI tests for JSON output and --check failing when not failover-ready. |
| docs/rfcs/0002-active-passive-fleet-ha.md | Update RFC to reflect loopback-only /health/ha and fleet-ha status tooling. |
| deployment-files/ha/tests/test-profile.sh | Add assertions that nginx blocks HA diagnostics through the VIP. |
| deployment-files/client/nginx.https.conf | Add nginx rule intended to block /api-proxy/health/ha through VIP. |
| deployment-files/client/nginx.http.conf | Add nginx rule intended to block /api-proxy/health/ha through VIP. |
Suppressed comments (1)
server/internal/ha/deployment/status.go:208
- The goroutine in this range closes over the loop variable
probe, so concurrent checks may all reference the same probe (typically the last one). That would break primary/replica counting and can misreport database readiness.
for _, probe := range probes {
go func() {
if endpointReadyWithClient(ctx, client, probe.endpoint) {
results <- probe.role
return
}
results <- ""
}()
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 372ade4542
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
372ade4 to
147dd0d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 147dd0d5f5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
64424fd to
c284fb4
Compare
- block all HA diagnostic path suffixes through nginx - open the writer probe through prepared database helpers
- use the host CA path for the writer probe - execute one sqlc query on one pinned connection
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b05b1e346
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
3392048 to
b2318ff
Compare
- block all HA diagnostic path suffixes through nginx - open the writer probe through prepared database helpers
- use the host CA path for the writer probe - execute one sqlc query on one pinned connection
a110845 to
131a6a6
Compare
Reviewable diff: +795/-78 across 16 files (excludes generated, test, and story files).
Summary
Gives operators a trustworthy answer to "is this HA cluster healthy, and would failover succeed right now?" without exposing cluster internals. Each Fleet host serves a redacted, local-only HA status document, and a new
fleet-ha statuscommand combines it with independent probes of etcd, Patroni, PostgreSQL, the VIP, and both Fleet hosts to produce a machine-readable readiness verdict. The public/healthresponse now carries the running release version in anX-Proto-Fleet-Versionheader, which the readiness check uses to detect mixed-version peers.Stack: This is PR 1 of 6, targeting
main: #887 -> #888 (install on clean Linux hosts) -> #889 (clean-install qualification) -> #890 (passive host updates) -> #891 (updates through bounded failover) -> #892 (adjacent-release update qualification). This PR only establishes operator visibility; installation, qualification, and updates land in the descendants, which usefleet-ha statusas their final readiness gate.How it works
Inside fleetd. The HA coordinator now tracks structured observation state instead of a free-text last error: whether an observation is available, and a
FreshUntilhorizon computed from the lease interval but never extending past the DCS proof deadline of the last writer observation. Losing the lease acquisition race to the peer is reclassified as a healthy outcome: a new SQL query (ClassifyFleetRuntimeLeaseAcquisition) distinguishes genuine contention from cluster identity or writer mismatches, and the coordinator records a contended acquisition as a valid passive observation after completing its closing DCS proof.Runtime.Statuscomposes this into the redacted operator contract: a coarse role (active,passive,initializing,degraded), observation freshness, endpoint health, and machine-readable reason codes. A passive node that still owns the VIP reportsdegraded, notpassive.Exposure boundaries. The status document is served at
/health/haonly when HA is enabled, and fleetd refuses to start in HA mode unless it listens on exactly127.0.0.1:4000, so the document cannot be exposed remotely by misconfiguration. The VIP nginx additionally returns 404 for/api-proxy/health/ha. Two signals are deliberately public through the VIP proxy:/healthgains theX-Proto-Fleet-Versionheader, and a new/health/passiveendpoint reports only whether the process is ready to take over (fresh observation, passive state, no VIP ownership).The CLI.
fleet-ha status [node.env]runs on ha-a or ha-b. It reads the local/health/hadocument, then runs five parallel control-path probes, each bounded to ~2 seconds, using the host's on-disk secrets and the read-onlyfleet-observeretcd credential:Observer.Observevalidation fleetd uses to acquire ownership (DCS leader, writable PostgreSQL identity, Patroni role and timeline agreement, live lease), over a fresh single pinned DB connection built from the on-diskDB_DSN(revalidated withValidateHA).The report derives
control_ready(the cluster works now) andfailover_ready(losing the active node would be survivable), each explained by reason codes (etcd_quorum_unavailable,writer_unavailable,fleet_version_mismatch, ...). Output is JSON; the command exits nonzero unlessfailover_ready, making it directly usable as a scripted gate.sequenceDiagram participant OP as Operator participant CLI as fleet-ha status participant FD as fleetd (127.0.0.1:4000) participant DEP as etcd / Patroni / PostgreSQL / VIP / peer Fleet OP->>CLI: run on ha-a or ha-b CLI->>FD: GET /health/ha FD-->>CLI: redacted runtime status CLI->>DEP: 5 parallel read-only probes (~2s each) DEP-->>CLI: per-dependency results CLI-->>OP: JSON report, exit 0 only if failover_readyAreas of the code involved
server/internal/ha/status.go(new)Statuscontract;Runtime.StatusandRuntime.Passivederivationserver/internal/ha/coordinator.goSnapshotdropsLastError, gainsObservationAvailable+FreshUntil; lease contention recorded as healthy observed passive after the closing DCS proofserver/internal/ha/store.go,server/sqlc/queries/ha.sqlClassifyFleetRuntimeLeaseAcquisitionquery maps a failed acquire tocontended/cluster_mismatch/writer_changed/unavailableIS DISTINCT FROMsemanticsserver/internal/ha/runtime.go,endpoint.go,config.goEndpointOwnedcheck wired into the runtime;LoadServiceTLSexported for host toolingserver/internal/handlers/health/handler.go/health/ha(redacted JSON),/health/passive,X-Proto-Fleet-Versionon/healthserver/cmd/fleetd/config.go,main.go127.0.0.1:4000; HA handlers registered only when HA is enabledserver/internal/ha/deployment/status.go(new, ~414 lines)statusengine: five parallel probes, readiness derivation, hardened probe HTTP client, generic fan-out helperserver/cmd/fleet-ha/main.gostatussubcommand: JSON output, nonzero exit unless failover-readyserver/internal/ha/deployment/preflight.govalidateFleetEnvironmentsplit so the status probe can reuseloadFleetEnvironmentdeployment-files/client/nginx.http.conf,nginx.https.conf/api-proxy/health/haserver/generated/sqlc/**docs/rfcs/0002-active-passive-fleet-ha.mdKey technical decisions & trade-offs
LastErrorwas removed fromSnapshotentirely so raw error strings (DSNs, hostnames) cannot reach any reportable surface.127.0.0.1:4000, and the VIP nginx 404s the path as a second layer, rather than relying on either alone.statusis a readiness gate, not a passive viewer: it always probes, always prints JSON, and exits nonzero unless failover-ready. There is no flag surface to keep qualified.failover_readyrequires both Fleet hosts to report the sameX-Proto-Fleet-Version, so a mid-upgrade cluster is never declared safe to fail over.fleet-observerrole.Testing & validation
ClassifyFleetRuntimeLeaseAcquisitionagainst a real database.deployment-files/ha/tests/test-profile.shasserts the nginx 404 for/api-proxy/health/ha.